UIU-3011 - UserInformation in UserDetails to display profile picture. #2618
Conversation
| */ | ||
| const isProfilePictureLinkAURL = !isAValidUUID(profilePictureLink); | ||
| const profilePicturesEnabled = Boolean(settings.length) && settings[0].enabled; | ||
| const hasViewProfilePicPerm = stripes.hasPerm('ui-users.profile-pictures.view'); |
There was a problem hiding this comment.
Please use Picture instead of Pic to ensure consistency with other places and make it easier to understand.
There was a problem hiding this comment.
in order not to make the variable name long, I have preferred pic to picture here. But if it seems to affect the readability, I can update it as per your suggestion.
| const hasProfilePicture = (settings.length && settings[0].value === 'true'); | ||
| const renderProfilePic = () => { | ||
| const profilePictureSrc = isProfilePictureLinkAURL ? profilePictureLink : 'data:;base64,' + profilePictureData; | ||
| const imgSrc = isLoading || isFetching || !hasProfilePicture ? ProfilePicThumbnail : profilePictureSrc; |
There was a problem hiding this comment.
Please select only the loading you need. Two are excessive.
There was a problem hiding this comment.
I will retain "isFetching" instead of isLoading as an image can take time to fetch based on its size.
| import { USER_TYPE_FIELD } from '../../../constants'; | ||
| import ProfilePicThumbnail from '../../../../icons/ProfilePicThumbnail.png'; |
There was a problem hiding this comment.
Please name the picture, starting with a lowercase letter.
| jest.mock('@folio/stripes/core', () => ({ | ||
| ...jest.requireActual('@folio/stripes/core'), | ||
| useStripes: jest.fn(() => ({ | ||
| hasPerm: () => true, | ||
| })), | ||
| })); |
There was a problem hiding this comment.
The useStripes is already mocked in stripesCore.mock.js.
| const useProfilePicture = ({ profilePictureId }, options = {}) => { | ||
| const ky = useOkapiKy(); | ||
| const [namespace] = useNamespace({ key: 'get-profile-picture-of-a-user' }); | ||
| const DEFAULT_DATA = {}; |
| jest.mock('@folio/stripes/core', () => ({ | ||
| ...jest.requireActual('@folio/stripes/core'), | ||
| useNamespace: jest.fn(() => ['test']), | ||
| useOkapiKy: jest.fn(), | ||
| })); |
There was a problem hiding this comment.
cleaned up several things
| // eslint-disable-next-line react/prop-types | ||
| const wrapper = ({ children }) => ( | ||
| <QueryClientProvider client={queryClient}> | ||
| {children} | ||
| </QueryClientProvider> | ||
| ); |
There was a problem hiding this comment.
Missed to clear it after updating code.
|
| records: 'configs', | ||
| path: 'configurations/entries?query=(module==USERS and configName==profile_pictures)', | ||
| path: 'users/configurations/entry', |
There was a problem hiding this comment.
@Terala-Priyanka, it appears this endpoint is added in users interface 16.1 but this query isn't fenced off by something that checks for that interface. This means the Q release either needed to drop support for 16 and bump its major version to 11, or this work should have been properly fenced off by interface version checks. Do you know which is correct?
Purpose
UIU-3011 - User Record View with Profile Picture
UIU-3024 - User Record View with profile picture thumbnail
Approach
This story implements changes to display user profile picture on user detail under "user information" accordion.
When profile pictures are enabled, and user has a profile picture, his/her profile picture is displayed. The source of the picture could be a public url or an id from the database. When in case of id(uuid), profile picture is fetched from API.
When profile pictures are enabled, and user does not have a profile picture, a thumbnail image will be displayed.
When profile pictures configuration is not enabled, then user detail will remain the same. The above-described changes will not reflect.
TODOS and Open Questions
Screencast
when profile pictures are enabled and user has permissions to view profile pictures-
https://github.com/folio-org/ui-users/assets/104053200/8cc64dee-ff01-4bab-ad4c-1495d1cc7ffa
Pre-Merge Checklist
Before merging this PR, please go through the following list and take appropriate actions.
If there are breaking changes, please STOP and consider the following:
Ideally all of the PRs involved in breaking changes would be merged in the same day to avoid breaking the folio-testing environment. Communication is paramount if that is to be achieved, especially as the number of intermodule and inter-team dependencies increase.
While it's helpful for reviewers to help identify potential problems, ensuring that it's safe to merge is ultimately the responsibility of the PR assignee.